-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support plugins from private Pulumi repos #9185
Conversation
This uses the same environment vars that we we're using for experimental private repos. That is GITHUB_TOKEN or GITHUB_ACTOR and GITHUB_PERSONAL_ACCESS_TOKEN. This allows us to develop plugins in private repos but continue to use our standard plugin flow.
@@ -196,47 +196,36 @@ type githubSource struct { | |||
name string | |||
kind PluginKind | |||
|
|||
tokenType string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tokenType
was always "token" so I've just trimmed it out
As described in https://docs.github.com/en/rest/overview/resources-in-the-rest-api#authentication we should be using the Authorization header.
cc @patriziobruno, you may want to test this is ok for your workflows. |
@@ -281,7 +282,11 @@ func newMockReadCloserString(data string) (io.ReadCloser, int64, error) { | |||
|
|||
//nolint:paralleltest // mutates environment variables | |||
func TestPluginDownload(t *testing.T) { | |||
expectedBytes := []byte{1, 2, 3} | |||
token := "RaNd0m70K3n_" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an actual token is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol no, but we don't actually care about the format of the token we just pass it straight through to the http header
if ghToken == "" { | ||
tokenType = "" | ||
paToken = os.Getenv("GITHUB_PERSONAL_ACCESS_TOKEN") | ||
actor = os.Getenv("GITHUB_ACTOR") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe consider warning if these env vars are detected as set? To help users transition that might rely on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GITHUB_PERSONAL_ACCESS_TOKEN is an envvar we made up so I can warn on that pretty safely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, very timely.
@Frassle, I love the new version, but it doesn't build on my machine and can't test it. I'm not sure I can use this workflow outside a github action without basic auth. |
I tested setting GITHUB_TOKEN to my personal access token and sending in the Authorization header and that let me download our private releases so I think this should work. This will be in the release this week so if it does break you just open an issue and we investigate quickly. |
@Frassie, I concur, I've just tried curling api.github.com with my personal access token used as GITHUB_TOKEN and it worked. |
Description
This makes a few changes to our github authentication flow.
This allows us to develop plugins in private repos but continue to use our standard plugin flow.
Checklist